Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revival #12

Merged
merged 37 commits into from
Mar 9, 2021
Merged

Revival #12

merged 37 commits into from
Mar 9, 2021

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Jan 27, 2021

Saw this feedstock didn't have an update in a while - it's going to become a dependency for allennlp 2.0.0, and I'm co-maintaining that feedstock

Closes #9
Closes #11

Possible follow-ups:

  • build with cuda
  • build with ffmpeg support
  • turn into multi-output recipe (e.g. with/without ffmpeg/image support)?
  • package c++ lib separately?
  • build for win/osx

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

Documentation on acceptable licenses can be found here.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari h-vetinari closed this Jan 28, 2021
@h-vetinari h-vetinari reopened this Jan 28, 2021
@h-vetinari
Copy link
Member Author

@isuruf @hmaarrfk @nehaljwani
This builds, runs the test-suite, and passes except for one py37-only failure (that I don't understand). Not sure if we want to have ffmpeg & cuda support already in this PR or as follow-ups, but I think this is pretty close.

recipe/meta.yaml Outdated
Comment on lines 59 to 60
# TODO: remove this once it is properly represented in pytorch build metadata
- typing_extensions # [py<38]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@h-vetinari
Copy link
Member Author

h-vetinari commented Jan 31, 2021

So my idea of patching this is not possible in this feedstock, since failing check is in CUDAExtension, which is not part of torchvision but comes from pytorch directly:

from torch.utils.cpp_extension import BuildExtension, CppExtension, CUDAExtension, CUDA_HOME

@hmaarrfk
Copy link
Contributor

do you want to monkey patch?

@h-vetinari
Copy link
Member Author

do you want to monkey patch?

Not sure what other options we'd have. Maybe @isuruf has some ideas?

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Feb 6, 2021

@jakirkham do you have any ideas on how to get around this? torch tries to determine gpu capabilities at build time.

@dirkgr
Copy link

dirkgr commented Feb 8, 2021

You probably already know this, but torch itself recommends installing torchvision with conda install pytorch torchvision -c pytorch or conda install pytorch torchvision cudatoolkit=11.0 -c pytorch. Their version works with and without GPUs, so there must be a way to compile it one way that works for both.

@h-vetinari
Copy link
Member Author

You probably already know this, but torch itself recommends installing torchvision with conda install pytorch torchvision -c pytorch or conda install pytorch torchvision cudatoolkit=11.0 -c pytorch. Their version works with and without GPUs, so there must be a way to compile it one way that works for both.

It's certainly possible to compile both in principle. What's not possible is using packages from outside conda-forge to build packages for conda-forge (with the sole exception of the anaconda default channels). Maybe we can get some inspiration from the pytorch recipes, provided they're licensed appropriately.

@jakirkham
Copy link
Member

do you have any ideas on how to get around this? torch tries to determine gpu capabilities at build time.

No idea. Would ask the PyTorch developers for guidance

@isuruf
Copy link
Member

isuruf commented Mar 9, 2021

@conda-forge-admin, rerender

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2021

Hi! This is the friendly automated conda-forge-webservice.
I tried to rerender for you, but it looks like there was nothing to do.

@isuruf
Copy link
Member

isuruf commented Mar 9, 2021

We can restart cuda 11 builds when pytorch cuda 11 builds are uploaded.

@isuruf isuruf merged commit b8ce1ae into conda-forge:master Mar 9, 2021
@h-vetinari h-vetinari deleted the revival branch March 9, 2021 23:23
@h-vetinari
Copy link
Member Author

Thanks for finishing this off, @isuruf!

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Mar 9, 2021

awesome!

@h-vetinari h-vetinari mentioned this pull request Mar 9, 2021
1 task
Comment on lines +4 to +18
export TORCH_CUDA_ARCH_LIST="3.5;5.0+PTX"
if [[ ${cuda_compiler_version} == 9.0* ]]; then
export TORCH_CUDA_ARCH_LIST="$TORCH_CUDA_ARCH_LIST;6.0;7.0"
elif [[ ${cuda_compiler_version} == 9.2* ]]; then
export TORCH_CUDA_ARCH_LIST="$TORCH_CUDA_ARCH_LIST;6.0;6.1;7.0"
elif [[ ${cuda_compiler_version} == 10.* ]]; then
export TORCH_CUDA_ARCH_LIST="$TORCH_CUDA_ARCH_LIST;6.0;6.1;7.0;7.5"
elif [[ ${cuda_compiler_version} == 11.0* ]]; then
export TORCH_CUDA_ARCH_LIST="$TORCH_CUDA_ARCH_LIST;6.0;6.1;7.0;7.5;8.0"
elif [[ ${cuda_compiler_version} == 11.1* ]]; then
export TORCH_CUDA_ARCH_LIST="$TORCH_CUDA_ARCH_LIST;6.0;6.1;7.0;7.5;8.0;8.6"
else
echo "unsupported cuda version. edit build.sh"
exit 1
fi
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isuruf
Shouldn't that +PTX be on the last available architecture?

@h-vetinari h-vetinari mentioned this pull request Mar 16, 2021
9 tasks
Comment on lines -14 to +21
skip: True # [not linux64]
script: python setup.py install --single-version-externally-managed --record=record.txt
number: {{ number }}
skip: true # [not linux]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting an anchor for easy referencing here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants